Skip to content

Porting SOF vBE#104

Closed
libinyang wants to merge 8 commits into
thesofproject:topic/sof-devfrom
libinyang:porting_1
Closed

Porting SOF vBE#104
libinyang wants to merge 8 commits into
thesofproject:topic/sof-devfrom
libinyang:porting_1

Conversation

@libinyang

Copy link
Copy Markdown

The first patch is from Yingjiang and Please ignore it. Thanks.

zhuyingjiang and others added 8 commits August 29, 2018 19:42
Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
This patch adds the sof virtio misc dev support. It will create a dev node
for the communication to the userspace.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch adds the sof support of open, ioctl and release for misc dev.
When open is called, sof creates sof_vbe for each virtual machine to manage
the vm.
When ioctl cmd shows vFE is ready, sof registers vhm client to manage the
vqs.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch initialize the virtio vq handling. It dispatches the different
vq kicks to different handlers. The specific hanlder will be implemented
later.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch constructs the basic flow of handling ipcs from SOF vFE.
The specific handling for each type of ipc will be implemented later.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch handles the tplg ipcs from SOF vFE.
vBE will create spcm when receiving the SOF_COMP_HOST ipc.
As mentioned in the patch, the next step is to create a whole
pcm staff, such as substream. With these, a whole pipeline can
be created when playback/capture.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch handles the stream ipcs from SOF vFE.
To save the resource, pcm open is handled in the hw_params cmd
and pcm close is handled in the hw_free cmd.

In this patch, hda/codec setting is handled with BE link substream.
This is safe as the BE substream is dedicated for the speical GOS PCM.
However, this is not the normal behavior. The normal behavior is setting
hda/codec in the FE link substream, which will be implemented later.

Signed-off-by: Libin Yang <libin.yang@intel.com>
This patch implements the position update for SOF vFE.

When there is position update from fw, vBE will check whether there is
an available entry in notification vq. If yes, send the position update
notification immediately. If there is no available entry, add the position
update event in a list. As soon as the notification vq has an available
entry, send the position update to the vFE.

Signed-off-by: Libin Yang <libin.yang@intel.com>

@lgirdwood lgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there. In genrel I need to see more comments. ideally one comment per code block that describes what the code does and why.

Comment thread sound/soc/sof/core.c
}

/* autosuspend sof device */
pm_runtime_mark_last_busy(sdev->dev);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need PM for BE and non virtio mode.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first patch is Yingjiang's patch. I use this as a code base. Please @zhuyingjiang help answer.

Comment thread sound/soc/sof/sof-priv.h
#include <sound/compress_driver.h>

#if IS_ENABLED(CONFIG_SND_SOC_SOF_VIRTIO_FE)
// TODO refine which ones are needed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has refinement of headers been done, if so we can remove.

Comment thread sound/soc/sof/sof-priv.h
struct list_head list;

// TODO: allow clients to have N streams. Map streams to posn[n]
struct sof_ipc_stream_posn pos[4];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best to define a macro here for 4.

Comment thread sound/soc/sof/sof-priv.h
int snd_sof_virtio_miscdev_register(struct snd_sof_dev *sdev);
int snd_sof_virtio_miscdev_unregister(void);
#else
static int snd_sof_virtio_miscdev_register(struct snd_sof_dev *sdev)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static inline

Comment thread sound/soc/sof/sof-priv.h
* VirtIO
*/
#define GUEST_COMP_ID_OFFSET 0x100
int sof_virtio_submit_guest_ipc(struct snd_sof_dev *sdev, int vm_id,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if these are private APIs to only be used within SOF then we can prefix them as sof_virtio_

Comment thread sound/soc/sof/virtio-fe.c
vdev->config->del_vqs(vdev);
kfree(vfe->posn);

// unregister the devices of SOF

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C style comments

return virtio_audio;
}

void *get_sof_dev(void)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix with sof_virtio_

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

return 0;
}

int snd_sof_virtio_miscdev_unregister(void)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean snd_sof_virtio_miscdev_unregister? For module mode, I think yes, we need it when unload the sof modules.

*
* Return: 0 for success or negative value for err
*/
int snd_audio_virtio_miscdev_register(struct device *dev, void *data,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we are only calling this internally ? if so it should be static.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I will change it.

Comment thread sound/soc/sof/virtio-be.c
static int sbe_pcm_open(struct snd_sof_dev *sdev,
void *ipc_data, int vm_id)
{
/*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stick this comment before func and comment each non obvious block in every function. i.e. I need to know what each block does.

@libinyang

Copy link
Copy Markdown
Author

Hi Liam, my first patch is copying from Yingjiang. I think he will submit the patch himself. I push it here because I use it as my code base. Otherwise, our patches will conflict.

@lgirdwood lgirdwood closed this Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet